Clean up mark_dirty() implementation to check for log-dirty

status internally, with the shadow_lock held for consistency.

Callers no need to check log-dirty status themselves.

Also, add dirty logging to alloc_page_type/free_page_type, so
that we see page tables coming and going during live relocation.

Signed-off-by: Keir Fraser <keir@xensource.com>

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 82d8093bd53a04a350da320da9c33fe66858b1aa..31ec92fe42a784a4fe2c25c4f4a393f9d5d07d2f 100644 (file)
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1289,6 +1289,11 @@ static int mod_l4_entry(l4_pgentry_t *pl4e,
 
 int alloc_page_type(struct pfn_info *page, unsigned long type)
 {
+    struct domain *owner = page_get_owner(page);
+
+    if ( owner != NULL )
+        mark_dirty(owner, page_to_pfn(page));
+
     switch ( type & PGT_type_mask )
     {
     case PGT_l1_page_table:
@@ -1318,16 +1323,14 @@ void free_page_type(struct pfn_info *page, unsigned long type)
     struct domain *owner = page_get_owner(page);
     unsigned long gpfn;
 
-    if ( owner != NULL )
+    if ( unlikely((owner != NULL) && shadow_mode_enabled(owner)) )
     {
+        mark_dirty(owner, page_to_pfn(page));
         if ( unlikely(shadow_mode_refcounts(owner)) )
             return;
-        if ( unlikely(shadow_mode_enabled(owner)) )
-        {
-            gpfn = __mfn_to_gpfn(owner, page_to_pfn(page));
-            ASSERT(VALID_M2P(gpfn));
-            remove_shadow(owner, gpfn, type & PGT_type_mask);
-        }
+        gpfn = __mfn_to_gpfn(owner, page_to_pfn(page));
+        ASSERT(VALID_M2P(gpfn));
+        remove_shadow(owner, gpfn, type & PGT_type_mask);
     }
 
     switch ( type & PGT_type_mask )
@@ -2203,8 +2206,7 @@ int do_mmu_update(
                     {
                         shadow_lock(d);
 
-                        if ( shadow_mode_log_dirty(d) )
-                            __mark_dirty(d, mfn);
+                        __mark_dirty(d, mfn);
 
                         if ( page_is_page_table(page) &&
                              !page_out_of_sync(page) )
@@ -2263,13 +2265,7 @@ int do_mmu_update(
             set_pfn_from_mfn(mfn, gpfn);
             okay = 1;
 
-            /*
-             * If in log-dirty mode, mark the corresponding
-             * page as dirty.
-             */
-            if ( unlikely(shadow_mode_log_dirty(FOREIGNDOM)) &&
-                 mark_dirty(FOREIGNDOM, mfn) )
-                FOREIGNDOM->arch.shadow_dirty_block_count++;
+            mark_dirty(FOREIGNDOM, mfn);
 
             put_page(&frame_table[mfn]);
             break;
@@ -2841,8 +2837,7 @@ long do_update_descriptor(u64 pa, u64 desc)
     {
         shadow_lock(dom);
 
-        if ( shadow_mode_log_dirty(dom) )
-            __mark_dirty(dom, mfn);
+        __mark_dirty(dom, mfn);
 
         if ( page_is_page_table(page) && !page_out_of_sync(page) )
             shadow_mark_mfn_out_of_sync(current, gpfn, mfn);

diff --git a/xen/arch/x86/shadow.c b/xen/arch/x86/shadow.c
index daad0b68d958f19e6c78cf97f5ff3c1ae6d1f142..c8ec7695960826229adeebd34979804676745025 100644 (file)
--- a/xen/arch/x86/shadow.c
+++ b/xen/arch/x86/shadow.c
@@ -1858,8 +1858,7 @@ static inline int l1pte_write_fault(
     SH_VVLOG("l1pte_write_fault: updating spte=0x%" PRIpte " gpte=0x%" PRIpte,
              l1e_get_intpte(spte), l1e_get_intpte(gpte));
 
-    if ( shadow_mode_log_dirty(d) )
-        __mark_dirty(d, gmfn);
+    __mark_dirty(d, gmfn);
 
     if ( mfn_is_page_table(gmfn) )
         shadow_mark_va_out_of_sync(v, gpfn, gmfn, va);
@@ -2021,9 +2020,7 @@ static int shadow_fault_32(unsigned long va, struct cpu_user_regs *regs)
             domain_crash_synchronous();
         }
 
-        // if necessary, record the page table page as dirty
-        if ( unlikely(shadow_mode_log_dirty(d)) )
-            __mark_dirty(d, __gpfn_to_mfn(d, l2e_get_pfn(gpde)));
+        __mark_dirty(d, __gpfn_to_mfn(d, l2e_get_pfn(gpde)));
     }
 
     shadow_set_l1e(va, spte, 1);
@@ -2082,8 +2079,7 @@ static int do_update_va_mapping(unsigned long va,
      * the PTE in the PT-holding page. We need the machine frame number
      * for this.
      */
-    if ( shadow_mode_log_dirty(d) )
-        __mark_dirty(d, va_to_l1mfn(v, va));
+    __mark_dirty(d, va_to_l1mfn(v, va));
 
     shadow_unlock(d);
 
@@ -3189,11 +3185,7 @@ static inline int l2e_rw_fault(
                 l1e_remove_flags(sl1e, _PAGE_RW);
             }
         } else {
-            /* log dirty*/
-            /*
-               if ( shadow_mode_log_dirty(d) )
-               __mark_dirty(d, gmfn);
-             */
+            /* __mark_dirty(d, gmfn); */
         }
        // printk("<%s> gpfn: %lx, mfn: %lx, sl1e: %lx\n", __func__, gpfn, mfn, l1e_get_intpte(sl1e));
         /* The shadow entrys need setup before shadow_mark_va_out_of_sync()*/
@@ -3476,9 +3468,7 @@ check_writeable:
         if (unlikely(!__guest_set_l1e(v, va, &gl1e))) 
             domain_crash_synchronous();
 
-        // if necessary, record the page table page as dirty
-        if ( unlikely(shadow_mode_log_dirty(d)) )
-            __mark_dirty(d, __gpfn_to_mfn(d, l2e_get_pfn(gl2e)));
+        __mark_dirty(d, __gpfn_to_mfn(d, l2e_get_pfn(gl2e)));
     }
 
     shadow_set_l1e_64(va, (pgentry_64_t *)&sl1e, 1);

diff --git a/xen/arch/x86/shadow32.c b/xen/arch/x86/shadow32.c
index bcbbdd5cc22721a0f1f6f05edb6aadf0fd8ef8f0..5636840f7078f1c02d9a581cf26fa4a80db97ed3 100644 (file)
--- a/xen/arch/x86/shadow32.c
+++ b/xen/arch/x86/shadow32.c
@@ -1274,8 +1274,6 @@ static int shadow_mode_table_op(
 
         d->arch.shadow_fault_count       = 0;
         d->arch.shadow_dirty_count       = 0;
-        d->arch.shadow_dirty_net_count   = 0;
-        d->arch.shadow_dirty_block_count = 0;
 
         break;
    
@@ -1284,13 +1282,9 @@ static int shadow_mode_table_op(
 
         sc->stats.fault_count       = d->arch.shadow_fault_count;
         sc->stats.dirty_count       = d->arch.shadow_dirty_count;
-        sc->stats.dirty_net_count   = d->arch.shadow_dirty_net_count;
-        sc->stats.dirty_block_count = d->arch.shadow_dirty_block_count;
 
         d->arch.shadow_fault_count       = 0;
         d->arch.shadow_dirty_count       = 0;
-        d->arch.shadow_dirty_net_count   = 0;
-        d->arch.shadow_dirty_block_count = 0;
  
         if ( (sc->dirty_bitmap == NULL) || 
              (d->arch.shadow_dirty_bitmap == NULL) )
@@ -1327,9 +1321,6 @@ static int shadow_mode_table_op(
     case DOM0_SHADOW_CONTROL_OP_PEEK:
         sc->stats.fault_count       = d->arch.shadow_fault_count;
         sc->stats.dirty_count       = d->arch.shadow_dirty_count;
-        sc->stats.dirty_net_count   = d->arch.shadow_dirty_net_count;
-        sc->stats.dirty_block_count = d->arch.shadow_dirty_block_count;
- 
 
         if ( (sc->dirty_bitmap == NULL) || 
              (d->arch.shadow_dirty_bitmap == NULL) )
@@ -2738,9 +2729,7 @@ int shadow_fault(unsigned long va, struct cpu_user_regs *regs)
             domain_crash_synchronous();
         }
 
-        // if necessary, record the page table page as dirty
-        if ( unlikely(shadow_mode_log_dirty(d)) )
-            __mark_dirty(d, __gpfn_to_mfn(d, l2e_get_pfn(gpde)));
+        __mark_dirty(d, __gpfn_to_mfn(d, l2e_get_pfn(gpde)));
     }
 
     shadow_set_l1e(va, spte, 1);
@@ -2837,8 +2826,6 @@ int shadow_do_update_va_mapping(unsigned long va,
 
     shadow_lock(d);
 
-    //printk("%s(va=%p, val=%p)\n", __func__, (void *)va, (void *)l1e_get_intpte(val));
-        
     // This is actually overkill - we don't need to sync the L1 itself,
     // just everything involved in getting to this L1 (i.e. we need
     // linear_pg_table[l1_linear_offset(va)] to be in sync)...
@@ -2853,10 +2840,8 @@ int shadow_do_update_va_mapping(unsigned long va,
      * the PTE in the PT-holding page. We need the machine frame number
      * for this.
      */
-    if ( shadow_mode_log_dirty(d) )
-        __mark_dirty(d, va_to_l1mfn(v, va));
+    __mark_dirty(d, va_to_l1mfn(v, va));
 
-// out:
     shadow_unlock(d);
 
     return rc;

diff --git a/xen/arch/x86/shadow_public.c b/xen/arch/x86/shadow_public.c
index 36454f9e51135093ef3b8a8c98fc43dc5f95ba63..cc91811e34c175d654b7f6736ae3384deb0a6c71 100644 (file)
--- a/xen/arch/x86/shadow_public.c
+++ b/xen/arch/x86/shadow_public.c
@@ -1183,8 +1183,6 @@ static int shadow_mode_table_op(
 
         d->arch.shadow_fault_count       = 0;
         d->arch.shadow_dirty_count       = 0;
-        d->arch.shadow_dirty_net_count   = 0;
-        d->arch.shadow_dirty_block_count = 0;
 
         break;
    
@@ -1193,15 +1191,10 @@ static int shadow_mode_table_op(
 
         sc->stats.fault_count       = d->arch.shadow_fault_count;
         sc->stats.dirty_count       = d->arch.shadow_dirty_count;
-        sc->stats.dirty_net_count   = d->arch.shadow_dirty_net_count;
-        sc->stats.dirty_block_count = d->arch.shadow_dirty_block_count;
 
         d->arch.shadow_fault_count       = 0;
         d->arch.shadow_dirty_count       = 0;
-        d->arch.shadow_dirty_net_count   = 0;
-        d->arch.shadow_dirty_block_count = 0;
  
-
         if ( (sc->dirty_bitmap == NULL) || 
              (d->arch.shadow_dirty_bitmap == NULL) )
         {
@@ -1236,8 +1229,6 @@ static int shadow_mode_table_op(
     case DOM0_SHADOW_CONTROL_OP_PEEK:
         sc->stats.fault_count       = d->arch.shadow_fault_count;
         sc->stats.dirty_count       = d->arch.shadow_dirty_count;
-        sc->stats.dirty_net_count   = d->arch.shadow_dirty_net_count;
-        sc->stats.dirty_block_count = d->arch.shadow_dirty_block_count;
  
         if ( (sc->dirty_bitmap == NULL) || 
              (d->arch.shadow_dirty_bitmap == NULL) )

diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index ed05d32c819eb9e9517cde3c4a181a0e67bcf2b3..94111fe1d12041e35177c5836dc8e0fb83b37ed4 100644 (file)
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -49,8 +49,6 @@ struct arch_domain
 
     unsigned int shadow_fault_count;
     unsigned int shadow_dirty_count;
-    unsigned int shadow_dirty_net_count;
-    unsigned int shadow_dirty_block_count;
 
     /* full shadow mode */
     struct out_of_sync_entry *out_of_sync; /* list of out-of-sync pages */

diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-x86/grant_table.h
index bd6eb09fc3d28379da868f54f1afab9a89ffbbef..38f46b69c9683493d1b5966d1a50a6ed4c3562bb 100644 (file)
--- a/xen/include/asm-x86/grant_table.h
+++ b/xen/include/asm-x86/grant_table.h
@@ -33,10 +33,6 @@ int steal_page_for_grant_transfer(
 #define gnttab_shared_mfn(d, t, i)                      \
     ((virt_to_phys((t)->shared) >> PAGE_SHIFT) + (i))
 
-#define gnttab_log_dirty(d, f)                          \
-    do {                                                \
-        if ( unlikely(shadow_mode_log_dirty((d))) )     \
-            mark_dirty((d), (f));                       \
-    } while ( 0 )
+#define gnttab_log_dirty(d, f) mark_dirty((d), (f))
 
 #endif /* __ASM_GRANT_TABLE_H__ */

diff --git a/xen/include/asm-x86/shadow.h b/xen/include/asm-x86/shadow.h
index 5d82713cae9aa190573dab6d52f81d11d6aab63d..d41303546869dcdf6c6bafd5149a8e7ba8870bd0 100644 (file)
--- a/xen/include/asm-x86/shadow.h
+++ b/xen/include/asm-x86/shadow.h
@@ -468,21 +468,18 @@ static inline void shadow_put_page(struct domain *d,
 
 /************************************************************************/
 
-static inline int __mark_dirty(struct domain *d, unsigned long mfn)
+static inline void __mark_dirty(struct domain *d, unsigned long mfn)
 {
     unsigned long pfn;
-    int           rc = 0;
 
     ASSERT(shadow_lock_is_acquired(d));
-    ASSERT(d->arch.shadow_dirty_bitmap != NULL);
 
-    if ( !VALID_MFN(mfn) )
-        return rc;
+    if ( likely(!shadow_mode_log_dirty(d)) || !VALID_MFN(mfn) )
+        return;
 
-    // N.B. This doesn't use __mfn_to_gpfn().
-    // This wants the nice compact set of PFNs from 0..domain's max,
-    // which __mfn_to_gpfn() only returns for translated domains.
-    //
+    ASSERT(d->arch.shadow_dirty_bitmap != NULL);
+
+    /* We /really/ mean PFN here, even for non-translated guests. */
     pfn = get_pfn_from_mfn(mfn);
 
     /*
@@ -491,16 +488,13 @@ static inline int __mark_dirty(struct domain *d, unsigned long mfn)
      * Nothing to do here...
      */
     if ( unlikely(IS_INVALID_M2P_ENTRY(pfn)) )
-        return rc;
+        return;
 
-    if ( likely(pfn < d->arch.shadow_dirty_bitmap_size) )
+    /* N.B. Can use non-atomic TAS because protected by shadow_lock. */
+    if ( likely(pfn < d->arch.shadow_dirty_bitmap_size) &&
+         !__test_and_set_bit(pfn, d->arch.shadow_dirty_bitmap) )
     {
-        /* N.B. Can use non-atomic TAS because protected by shadow_lock. */
-        if ( !__test_and_set_bit(pfn, d->arch.shadow_dirty_bitmap) )
-        {
-            d->arch.shadow_dirty_count++;
-            rc = 1;
-        }
+        d->arch.shadow_dirty_count++;
     }
 #ifndef NDEBUG
     else if ( mfn < max_page )
@@ -513,18 +507,17 @@ static inline int __mark_dirty(struct domain *d, unsigned long mfn)
                frame_table[mfn].u.inuse.type_info );
     }
 #endif
-
-    return rc;
 }
 
 
-static inline int mark_dirty(struct domain *d, unsigned int mfn)
+static inline void mark_dirty(struct domain *d, unsigned int mfn)
 {
-    int rc;
-    shadow_lock(d);
-    rc = __mark_dirty(d, mfn);
-    shadow_unlock(d);
-    return rc;
+    if ( unlikely(shadow_mode_log_dirty(d)) )
+    {
+        shadow_lock(d);
+        __mark_dirty(d, mfn);
+        shadow_unlock(d);
+    }
 }
 
 
@@ -566,8 +559,7 @@ __guest_set_l2e(
     if ( unlikely(shadow_mode_translate(d)) )
         update_hl2e(v, va);
 
-    if ( unlikely(shadow_mode_log_dirty(d)) )
-        __mark_dirty(d, pagetable_get_pfn(v->arch.guest_table));
+    __mark_dirty(d, pagetable_get_pfn(v->arch.guest_table));
 }
 
 static inline void
@@ -787,8 +779,7 @@ static inline int l1pte_write_fault(
     SH_VVLOG("l1pte_write_fault: updating spte=0x%" PRIpte " gpte=0x%" PRIpte,
              l1e_get_intpte(spte), l1e_get_intpte(gpte));
 
-    if ( shadow_mode_log_dirty(d) )
-        __mark_dirty(d, gmfn);
+    __mark_dirty(d, gmfn);
 
     if ( mfn_is_page_table(gmfn) )
         shadow_mark_va_out_of_sync(v, gpfn, gmfn, va);
@@ -1325,46 +1316,6 @@ shadow_max_pgtable_type(struct domain *d, unsigned long gpfn,
     return pttype;
 }
 
-/*
- * N.B. We can make this locking more fine grained (e.g., per shadow page) if
- * it ever becomes a problem, but since we need a spin lock on the hash table 
- * anyway it's probably not worth being too clever.
- */
-static inline unsigned long get_shadow_status(
-    struct domain *d, unsigned long gpfn, unsigned long stype)
-{
-    unsigned long res;
-
-    ASSERT(shadow_mode_enabled(d));
-
-    /*
-     * If we get here we know that some sort of update has happened to the
-     * underlying page table page: either a PTE has been updated, or the page
-     * has changed type. If we're in log dirty mode, we should set the
-     * appropriate bit in the dirty bitmap.
-     * N.B. The VA update path doesn't use this and is handled independently. 
-     *
-     * XXX need to think this through for vmx guests, but probably OK
-     */
-
-    shadow_lock(d);
-
-    if ( shadow_mode_log_dirty(d) )
-        __mark_dirty(d, __gpfn_to_mfn(d, gpfn));
-
-    if ( !(res = __shadow_status(d, gpfn, stype)) )
-        shadow_unlock(d);
-
-    return res;
-}
-
-
-static inline void put_shadow_status(struct domain *d)
-{
-    shadow_unlock(d);
-}
-
-
 static inline void delete_shadow_status(
     struct domain *d, unsigned long gpfn, unsigned long gmfn, unsigned int stype)
 {